GraphClient and GraphRequestAdapter#555
Conversation
ramsessanchez
requested review from
MIchaelMainer,
baywet,
ddyett,
nikithauc and
zengin
as code owners
src/main/java/com/microsoft/graph/Requests/BaseGraphRequestAdapter.java
Outdated
Show resolved
Hide resolved
src/main/java/com/microsoft/graph/Requests/BaseGraphRequestAdapter.java
Outdated
Show resolved
Hide resolved
src/main/java/com/microsoft/graph/Requests/BaseGraphRequestAdapter.java
Outdated
Show resolved
Hide resolved
src/main/java/com/microsoft/graph/Requests/GraphClientFactory.java
Outdated
Show resolved
Hide resolved
…pter.java Co-authored-by: Vincent Biret <vibiret@microsoft.com>
Add overloads to BaseClient
Update target jdk to 17
|
Spotbugs failing, this is expected. |
|
Added finalized constructors for baseClient and baseGraphRequestAdapter with following param patterns: |
src/main/java/com/microsoft/graph/Requests/GraphClientFactory.java
Outdated
Show resolved
Hide resolved
| /** | ||
| * Default client implementation. | ||
| */ | ||
| public class BaseClient implements IBaseClient{ |
There was a problem hiding this comment.
I don't believe we can have a base client (class) in core anymore.
That's because the GeneratedGraphServiceClient will be a class that doesn't inherit from this one and Java doesn't support multiple inheritance.
So this will either have to move to the service library and inherit from the generated client or be removed all together.
There was a problem hiding this comment.
Hey Vincent, it seems that in C# there is this baseClient class as a means to create a lightweight client that doesn't rely on the service library. I believe we can keep this class here as GraphServiceClient will extend from BaseGraphServiceClient and implement IBaseClient. This is the same pattern that we have in C#. This means I will have to create the GraphServiceClient following the same pattern that is already in C#, but I believe we can keep both.
There was a problem hiding this comment.
alright, but just to be clear, this one won't be in the inheritance structure of a client for somebody using the graph service client from the service lib, correct?
There was a problem hiding this comment.
no, the the GraphServiceClient will extend the BaseGraphServiceClient and implement IBaseClient, GraphServiceClient will not use any BaseClient implementations.
There was a problem hiding this comment.
so should this class be removed from the PR?
src/main/java/com/microsoft/graph/Requests/GraphClientFactory.java
Outdated
Show resolved
Hide resolved
| customizePom(pom) | ||
| groupId project.property('mavenGroupId') | ||
| artifactId project.property('mavenArtifactId') | ||
| version "${mavenMajorVersion}.${mavenMinorVersion}.${mavenPatchVersion}${mavenCentralSnapshotArtifactSuffix}" | ||
| from components.java | ||
| pom.withXml { | ||
| def pomFile = file("${project.buildDir}/generated-pom.xml") | ||
| writeTo(pomFile) | ||
| } | ||
| artifact sourceJar | ||
| artifact javadocJar | ||
| } | ||
|
|
||
| mavenCentralRelease(MavenPublication) { | ||
| customizePom(pom) | ||
| groupId project.property('mavenGroupId') | ||
| artifactId project.property('mavenArtifactId') | ||
| version "${mavenMajorVersion}.${mavenMinorVersion}.${mavenPatchVersion}" | ||
| from components.java | ||
| pom.withXml { | ||
| def pomFile = file("${project.buildDir}/generated-pom.xml") | ||
| writeTo(pomFile) | ||
| } | ||
| artifact sourceJar | ||
| artifact javadocJar | ||
| } |
There was a problem hiding this comment.
minor issue: @ramsessanchez @baywet We can consider configuring a deterministic code formatter to avoid diff due to indenting and prevent merging code that's not formatted al together.
I've worked with spotless which can be configured to check PR's and can be used a maven / gradle plugin. For the formatting rules we can use a derivative of google-formatter
There was a problem hiding this comment.
go for it! I usually rely on editor config, but as long as it works on vs code I'm happy. Don't hesitate to pr microsoft/kiota-java (no need to PR the service libs)
3 participants
Fixes #533 #537
GraphClientFactory
BaseGraphRequestAdapter